Skip to content

ENH: add na_action to Categorical.map & CategoricalIndex.map #51645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 16 commits into from
Mar 30, 2023
Merged

ENH: add na_action to Categorical.map & CategoricalIndex.map #51645

merged 16 commits into from
Mar 30, 2023

Conversation

topper-123
Copy link
Contributor

@topper-123 topper-123 force-pushed the categorical_map_na_Action branch from 5674674 to 10f8606 Compare March 4, 2023 21:47
@topper-123 topper-123 mentioned this pull request Mar 6, 2023
Merged
1 task
@topper-123 topper-123 force-pushed the categorical_map_na_Action branch 2 times, most recently from 6b461b2 to 64b7297 Compare March 14, 2023 18:44
@mroeschke mroeschke added the Categorical Categorical Data Type label Mar 17, 2023
mroeschke
mroeschke reviewed Mar 17, 2023
View reviewed changes
@topper-123
Copy link
Contributor Author

I've updated again. I noticed that the implicit default for na_action is "ignore" for Categorical.map, while it's None generally. This is a bit of an issue because e.g. Series.map has a default of None and that can't be changed, so there's some mismatch when calling the Categorical directly compared to doing it through Series.map.

So I've set the default value of na_action="ignore" in Categorical.map as being deprecated and set to be changed in the next version.

@topper-123 topper-123 force-pushed the categorical_map_na_Action branch from 7775e2b to dc60e20 Compare March 22, 2023 05:05
@topper-123 topper-123 mentioned this pull request Mar 22, 2023
Closed
@topper-123 topper-123 force-pushed the categorical_map_na_Action branch from 1f19c3e to dc9d119 Compare March 24, 2023 16:28
@topper-123
Copy link
Contributor Author

I’ve simplified the implementation.

@topper-123 topper-123 force-pushed the categorical_map_na_Action branch from 1fc89b7 to be2d451 Compare March 29, 2023 07:04
@topper-123
Copy link
Contributor Author

Ping.

mroeschke
mroeschke reviewed Mar 29, 2023
View reviewed changes
pandas/core/apply.py Outdated
mapped = obj._map_values(mapper=f, convert=self.convert_dtype)
# apply doesn't have a `na_action` keyword and for backward compat reasons
# we need to give `na_action="ignore"` for categorical data.
# TODO: remove the `na_action="ignore"` has been removed from Categorical.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to refer to once the deprecation is enforced?

mroeschke
mroeschke reviewed Mar 29, 2023
View reviewed changes
pandas/core/arrays/categorical.py Outdated

.. deprecated:: 2.1.0

The dault value of 'ignore' has been deprecated and will be changed to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The dault value of 'ignore' has been deprecated and will be changed to
The default value of 'ignore' has been deprecated and will be changed to

mroeschke
mroeschke reviewed Mar 30, 2023
View reviewed changes
Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic looks a lot simpler thanks. Kinda unfortunate that user will automatically get a deprecation warning, but i think this is okay as map is a fairly uncommon method

@mroeschke mroeschke added this to the 2.1 milestone Mar 30, 2023
@mroeschke mroeschke merged commit 72cd001 into pandas-dev:main Mar 30, 2023
@mroeschke
Copy link
Member

Thanks @topper-123

@topper-123 topper-123 deleted the categorical_map_na_Action branch March 30, 2023 16:29
topper-123 added a commit to topper-123/pandas that referenced this pull request Apr 1, 2023
@topper-123
ENH: add na_action to Categorical.map & CategoricalIndex.map (pandas-…
8cdef8a 
…dev#51645)

* ENH: add na_action to Categorical.map

* add GH numbers

* pre-commit issues

* map Categorical with Series

* REF: simplify .map

* pass test_map

* fix whatsnew

* cleanups

* pre-commit

* deprecate Categorical.map(na_action=ignore)

* fix docstrings

* fix rebase

* simplity implementation

* fix warn

* fix comments
@mroeschke mroeschke mentioned this pull request May 7, 2024
Merged
@rhshadrach rhshadrach mentioned this pull request Oct 2, 2024
Closed
3 tasks
This was referenced Oct 2, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke approved these changes

Assignees
No one assigned
Labels
Categorical Categorical Data Type
Projects
None yet
Milestone
2.1
2 participants
@topper-123 @mroeschke